Repository navigation
Hide native Windows 11 Start button when using a replacement - #2547
YellowNest wants to merge 34 commits into
Conversation
|
This is super awesome. Glad to see it works nicely. And I guess it should be used for more customizations and improvements related to start button and taskbar itself. Do you plan to work more in this area? I think it would be really appreciated. |
|
I have noticed that enabling custom Start button cause also that TaskView button dissapears. There is just empty place instead of the button. |
|
Another issue is that when I Starting |
|
Thanks for testing this carefully. Both reports were valid and pointed to separate problems in the initial TAP implementation. I pushed the follow-up in The Task View regression came from treating every The restart problem was a separate TAP lifetime issue. Shutdown now restores the XAML properties synchronously, removes the visual-tree callback with The build for the follow-up is here: The two paths that still deserve runtime confirmation are:
And yes, I plan to keep working in this area. Replacing |
Definitely. I'm glad you plan to work on this more. |
|
Thanks, I really appreciate that. I use Open-Shell myself, so contributing here is useful to me too. I care about keeping it working well on current Windows builds, and if I can help take some of the load off, I'm happy to. I also appreciate the trust you're putting in the work. Your reviews and testing are valuable, especially around these Windows 11 edge cases, so I'll keep changes focused and separate like you suggested. I've already started looking at the XAML input side in a separate research branch. The first build is clean, but I'll keep it out of upstream until I've tested the behavior properly. Glad to help. |
I can confirm this works properly now.
I'm now experiencing random Explorer crashes when Exiting Open-Shell and starting it again (using Open-Shell Menu Settings shortcut). This is on 26H2 (Windows Sandbox). I can share the dump eventually. |
|
Thanks for the dump trace. I treated this as a teardown/lifetime problem rather than an accessibility-code failure and reworked the TAP shutdown path. The latest head is The important changes are:
I also checked the current upstream master changes; they don't overlap this code. The full upstream PR build for The critical runtime test now is repeated cycles on 26H2:
If Explorer still crashes, the full dump would be useful. I don't want to paper over this with another timing workaround; the remaining failure, if any, should be traced from the dump. |
Bundle current master with pending PRs Open-Shell#2547, Open-Shell#2548, Open-Shell#2551, Open-Shell#2552, and Open-Shell#2553 for local integration testing. This branch is test packaging only and is not intended for upstream merge.
2fb59e7 to
192b2e3
Compare
|
|
||
| HRESULT Deactivate( void ) | ||
| { | ||
| // The diagnostics runtime retains this site beyond Open-Shell's lifetime. |
There was a problem hiding this comment.
I think we may need to call m_Visual->UnadviseVisualTreeChange here.
It seems that when I exit Open-Shell, StartMenuHelper64.dll stays loaded in explorer. Something is still holding reference to it. I think it is the XAML diagnostic framework.
CWin11StartButtonTap destructor is never called (I have breakpoint there).
There was a problem hiding this comment.
Addressed in bee430c. Exit now restores our XAML overrides synchronously before unregistering the callback, then calls UnadviseVisualTreeChange, clears the replay state, drains queued apply work and tears down the dispatch window.
One distinction is intentional: StartMenuHelper64.dll may still remain mapped after a successful InitializeXamlDiagnosticsEx. The public XAML diagnostics surface has no session-uninitialize API; UnadviseVisualTreeChange only unregisters visual-tree mutation callbacks. Forcing the injected module out while the diagnostics runtime still owns the TAP object would be unsafe. Open-Shell-owned subscription/window/state is now fully detached on Exit, while Start can re-advise the retained diagnostics service without creating another diagnostics session.
The runtime path worth retesting is repeated Exit -> Open-Shell Menu Settings restart on 26H2.
There was a problem hiding this comment.
The public XAML diagnostics surface has no session-uninitialize API; UnadviseVisualTreeChange only unregisters visual-tree mutation callbacks.
It seems that after UnadviseVisualTreeChange the XAML framework frees TAP object because CWin11StartButtonTap destructor is called.
Yet, StartMenuHelper64.dll stays loaded. I'm not sure what is holding it. But normally (in actual Open-Shell) it is unloaded after some time (like few minutes or so).
I was also trying to use UWPSpy that also uses TAP mechanism. It has DLL that is loaded into examined process and when you close the tool, the DLL gets unloaded.
I mean this is probably no big deal, because normally one doesn't exit Open-Shell.
It may just cause issues during update, where we will now need to restart Explorer for sure (previously it was not always necessary).
|
The crash that I mentioned in #2547 (comment) is actually not related to changes in this PR. I can replicate it with official So far have no clue what could be wrong. |
ge0rdi
left a comment
ge0rdi
left a comment
There was a problem hiding this comment.
I'm sorry. but I have feeling like it gets more complicated with each batch of changes :(
I'd rather keep it simpler (as the idea is rather simple).
I'll try to provide some more comments, just don't have energy for it now.
| ~CWin11StartButtonTap( void ) | ||
| { | ||
| { | ||
| std::unique_lock lock(g_TapMutex); |
There was a problem hiding this comment.
This doesn't feel right. We should rather remove g_Tap reference when we are deactivating TAP (Deactivate method or StopWin11StartButtonTap perhaps).
| } | ||
|
|
||
| { | ||
| std::lock_guard lock(m_LifecycleMutex); |
There was a problem hiding this comment.
Here we are in object destructor. That means no other threads are supposed to use this object (otherwise the behavior would be undefined).
So there is no need to hold object mutexes here.
Moreover it makes no sense to call Unadvise here - we had to call it before, otherwise we won't get here.
Also no point to ResetElements - nobody can use those anymore.
We should just destroy dispatch window and that's it.
| // Never leave a window pointing at an object that is being destroyed. | ||
| HWND dispatch = m_Dispatch; | ||
| if (dispatch) | ||
| SetWindowLongPtr(dispatch, GWLP_USERDATA, 0); |
There was a problem hiding this comment.
I think this belongs to DestroyDispatchWindow. That function is responsible for destroying the window so one of first things it should do is to reset user data.
Here in destructor we should just call DestroyDispatchWindow and that's it.
| return false; | ||
| } | ||
|
|
||
| HRESULT RequestApply( bool synchronous, bool enabled ) |
There was a problem hiding this comment.
This functions looks overly complicated.
It is supposed to just pass some message.
Why to complicate it with all those error handlings and timeouts.
This branch has not been deployed
2 participants
Summary
Hide the native Windows 11 Start button visuals while Open-Shell's replacement Start button is enabled.
Windows 11 renders the Start button through XAML, so the older HWND-based hiding logic cannot remove the native glyph. This caused custom Open-Shell buttons to be drawn on top of the Windows Start icon.
The change uses the Windows XAML diagnostics visual tree to:
All taskbarssettingThe replacement button itself is not moved or resized by this code, so Windows remains responsible for centered taskbar layout.
Verification
Tested on Windows 11 25H2 with centered taskbar:
Replace Start buttonrestores the native Windows Start button.GitHub Actions
Buildcompleted successfully for the current heade9039e3:https://github.com/Open-Shell/Open-Shell-Menu/actions/runs/37805091803
The lifecycle implementation avoids loading StartMenuHelper and initializing XAML Diagnostics when the replacement Start button is disabled.
Repeated Explorer exit/restart and complete injected DLL unloading still require runtime verification. A successful build does not establish those behaviors.
Fixes #1631